-
-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pandas 1.3.0 compatibility #49
Conversation
@@ -129,7 +141,7 @@ def block_to_header_bytes(block): | |||
elif is_datetime64tz_dtype(block): | |||
extension = ('datetime64_tz_type', (block.values.tzinfo,)) | |||
values = values.view('i8') | |||
elif is_extension_array_dtype(block.dtype): | |||
elif is_extension_array_dtype(block.dtype) or is_extension_array(values): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose you could also replace this with elif isinstance(values, ExtensionArray)
, and I would think this should work for older versions of pandas as well.
(but the current PR should work as well, I think)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reviewing @jorisvandenbossche! I avoided a pure isinstance(values, ExtensionArray)
check since partd
supports older versions of pandas
which predate the ExtensionArray
class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, sounds good then!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could also do a not isinstance(values, np.ndarray)
(unless you support pandas so old that it can still be a CategoricalIndex or something)
@jrbourbeau do you plan to do a bugfix release with this change? |
Yes. I'm checking that this resolves the issue reported in |
Opened up an issue for releasing over at #51 |
This PR adds support for
pandas
1.3.0. cc @jorisvandenbossche if you get a chance to take a lookCloses #48, xref dask/dask#7507